Fix #admin room visibility in LHN for Track Workspace onboarding - #94104
Conversation
|
@linhvovan29546 Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button] |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5a10a3e3bb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| isFocused || | ||
| isSystemChat || | ||
| !!report.isPinned || | ||
| isAdminRoom(report) || |
There was a problem hiding this comment.
Restrict admin-room hidden override to chat reports
This exemption applies to any report whose chatType is policyAdmins, not just the root #admins room. Admin-room task reports intentionally inherit that chat type for routing (buildOptimisticTaskReport in src/libs/ReportUtils.ts), and those task reports are created with notificationPreference: hidden; after this change, completed/non-actionable tasks under #admins bypass the hidden-report gate and fall through to the default LHN inclusion path. Please limit the override to actual chat rooms, e.g. isChatReport(report) && isAdminRoom(report), so hidden child task reports do not appear in the LHN.
Useful? React with 👍 / 👎.
|
|
|
@emkhalid Could you please merge main? and resolve the PR Author Checklist https://github.com/Expensify/App/actions/runs/27855249389/job/82441406574?pr=94104 |
Thanks for the review, I merged the main. |
There was a problem hiding this comment.
Could you please revert the changes in this file? I don’t think this PR needs them.
| isSelfTourViewed, | ||
| }); | ||
|
|
||
| if (!isSidePanelReportSupported) { |
There was a problem hiding this comment.
Why do we need this change?
There was a problem hiding this comment.
@linhvovan29546 Thanks for the review.
I left the useAutoCreateTrackWorkspace.ts change for now because in testing the fix worked on web, but native still reproduced until I added the native-only wait after CompleteGuidedSetup. It seems native navigates before the write queue is fully settled.
If you want this PR kept strictly to the approved SidebarUtils/ReportUtils scope, I’m happy to remove that hook change and treat native as a separate follow-up.
There was a problem hiding this comment.
@emkhalid Hmm, I can't reproduce the bug you mentioned.
Screen.Recording.2026-06-26.at.16.54.00.mov
There was a problem hiding this comment.
@linhvovan29546 Thanks for the review, and apologies for the confusion.
I re-tested after reverting the useAutoCreateTrackWorkspace.ts change, and you were right — the fix works without it. I likely tested it incorrectly earlier and assumed that native-only wait was needed when it wasn’t.
I’ve reverted that hook change so this PR is now scoped only to the approved fix.
Reviewer Checklist
Screenshots/VideosAndroid: HybridApptelegram-cloud-document-5-6192769913669558441.mp4Android: mWeb Chrometelegram-cloud-document-5-6192769913669558442.mp4iOS: HybridAppScreen.Recording.2026-06-26.at.17.24.57.moviOS: mWeb SafariScreen.Recording.2026-06-26.at.16.54.00.movMacOS: Chrome / SafariScreen.Recording.2026-06-26.at.17.22.39.mov |
|
@emkhalid Could you please add a unit test? |
Thanks for the review, Done. |
|
🚧 lakchote has triggered a test Expensify/App build. You can view the workflow run here. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🚀 Deployed to staging by https://github.com/lakchote in version: 9.4.21-2 🚀
|
|
Deploy Blocker #94747 was identified to be related to this PR. |
|
Deploy Blocker #94781 was identified to be related to this PR. |
|
🚀 Deployed to production by https://github.com/mountiny in version: 9.4.21-9 🚀
Bundle Size Analysis (Sentry): |
Explanation of Change
This PR fixes the Track Workspace onboarding flow so the workspace
#adminroom stays visible in the default LHN.The fix is intentionally scoped to the two gating conditions identified in the approved proposal:
SidebarUtilsReportUtilsThis preserves the Concierge routing introduced by PR #82804 while restoring the expected LHN visibility for the
#adminroom.Fixed Issues
$ #92431
PROPOSAL: #92431 (comment)
Tests
+in the email.Track expenses for my business.#adminroom appears in the default LHN without needing search.#adminroom.#adminroom also appears in the default LHN without needing search.Offline tests
#adminroom is visible.#adminroom remains accessible from the LHN.QA Steps
Same as Tests
// TODO: These must be filled out, or the issue title must include "[No QA]."
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)Avatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))npm run compress-svg)Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
android-native-admin.mp4
Android: mWeb Chrome
android-web-admin.mp4
iOS: Native
ios-native-admin.mp4
iOS: mWeb Safari
ios-web-admin.mp4
MacOS: Chrome / Safari
macos-admin.mp4